Skip to content

Conversation

fkwp
Copy link
Contributor

@fkwp fkwp commented Aug 14, 2025

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • I have licensed the changes to Element by completing the Contributor License Agreement (CLA)

Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to match anything written in MSC3401?

The MSC says the state key to be the user ID, no suffix.

Users who want to participate in the call declare this by publishing a m.call.member state event using their matrix ID as the state key (thus ensuring other users cannot edit it).

@fkwp
Copy link
Contributor Author

fkwp commented Aug 14, 2025

This doesn't seem to match anything written in MSC3401?

The MSC says the state key to be the user ID, no suffix.

Users who want to participate in the call declare this by publishing a m.call.member state event using their matrix ID as the state key (thus ensuring other users cannot edit it).

agreed that is a bit weird also for MSC4143 we are using the msc3401 prefix...

This is needed to allow multiple MatrixRTC sessions in parallel, e.g., Element Call and a the Nordeck Neo Board

See also:

@t3chguy
Copy link
Member

t3chguy commented Aug 14, 2025

Changing the semantics of something defined by another MSC really isn't ideal.

This means breaking support for one MSC whilst gaining support for another, whilst clobbering unstable IDs? I can't say this is great...

Especially given state events are permanent. They can never be removed from a room, and they will clobber sync responses into the future, and be forever held in memory via the sync accumulator going from O(n) to O(n^2)

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it's definitely pretty counterintuitive that MSC4143 is using the unstable prefix of MSC3401 and I would rather we had migrated to the proper org.matrix.msc4143.* namespace already, the fact is that both MSCs are championed and coordinated by the same team and so we can afford a bit of namespace encroachment. (MSC3401 should be closed soon, actually…) Hopefully we can update the MatrixRTC MSCs to reflect that they use this unstable prefix, since they are effectively just iterations of the previous group VoIP concept.

And the permanence of these entries in room state is unfortunately just part of the deal for now. It's always been planned that there could be a couple changes to the event types or state keys along the path to stability for these features, for example with the removal of the underscore prefix. I do hope we can get more momentum on state deletion or device-specific room state concepts eventually.

@robintown robintown dismissed t3chguy’s stale review August 15, 2025 19:18

Overriding as per my above review - Florian tells me Michael expressed that a second opinion approving the PR would be okay.

@fkwp fkwp added this pull request to the merge queue Aug 15, 2025
Merged via the queue into develop with commit aa5bdab Aug 15, 2025
35 checks passed
@fkwp fkwp deleted the fkwp/element-call/new_state_key_format branch August 15, 2025 19:37
Dileep9999 pushed a commit to hemanth-nag/element-web that referenced this pull request Oct 8, 2025
…ix (element-hq#30566)

* Extended string-packing for state keys allowing additonially the `_m.call` suffix

* add comment about unstable prefix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Task Tasks for the team like planning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants